fix(connection): reject pending callbacks on transport death - #3170
fix(connection): reject pending callbacks on transport death#3170Simon Knott (Skn0tt) wants to merge 1 commit into
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3351313b-bb35-475c-ae04-f79e1d2e4086
| # relies on it to attribute rejections to the right test. | ||
| if browser: | ||
| connection._loop.call_soon(browser._on_close) | ||
| connection.cleanup(reason) |
There was a problem hiding this comment.
this seems like it'd reintroduce the problem solved by #3135
There was a problem hiding this comment.
Hmm, the test added is still green though. Can you explain how?
There was a problem hiding this comment.
that test stays green because page._on_close() synchronously rejects its local Waiter before browser._on_close is queued
this change queues browser._on_close before connection.cleanup() rejects its callback, so disconnected can run first
a pending page.evaluate("new Promise(() => {})") would expose the ordering regression
| }, | ||
| return_when=asyncio.FIRST_COMPLETED, | ||
| ) | ||
| result = await callback.future |
There was a problem hiding this comment.
this seems like it'd reintroduce the problem solved by #3144
There was a problem hiding this comment.
Can you elaborate on how? Task cancellation still triggers __abort__, which ends the request on the server side and eventually makes this callback reject.
There was a problem hiding this comment.
the distinction is that __abort__ is still sent, but the caller no longer waits for it to complete
since callback.future is directly awaited, canceling the task also cancels that future
_abort() then sends __abort__, awaits the already canceled future, catches CancelledError immediately, and returns
the later server response is discarded by the callback.future.cancelled() branch in dispatch(), rather than making the callback reject
the existing test stays green because it checks the eventual effect of sending __abort__, not whether cancellation waits for the protocol response
2 participants
Every send used to race the protocol callback against
transport.on_error_future, because a dead pipe only completed that future and never tore down in-flight calls.This matches the Node client more closely:
Connection.run()callscleanup()when the transport ends, which rejects every pending callback. Sends just await their own future.For remote connect,
browser._on_closeis stillcall_soon'd beforecleanup(), so waiters still see reject-before-disconnect ordering.